Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standardize "use parentheses to call" suggestions between typeck and trait selection #102863

Merged

Conversation

compiler-errors
Copy link
Member

  1. Suggest calling constructors, since they're basically FnDefs but they have a different def kind and hir representation, so we were leaving them out.
  2. Standardize the call suggestions between trait fulfillment errors and type mismatch. In the type mismatch suggestion, we suggest /* Ty */ as the placeholder for an arg, and not the parameter's name, which is less helpful.
  3. Use predicate_must_hold_modulo_regions instead of matching on EvaluationResult -- this might cause some suggestions to be filtered out, but we really shouldn't be suggesting a call if it "may" hold, only when it "must" hold.
  4. Borrow some logic from extract_callable_info to generalize this suggestion to fn pointers, type parameters, and opaque types.

Fixes #102852

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 9, 2022
@rust-highfive
Copy link
Collaborator

r? @nagisa

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 9, 2022
@@ -11,6 +11,10 @@ note: required by a bound in `take_const_owned`
|
LL | fn take_const_owned<F>(_: F) where F: FnOnce() + Sync + Send {
| ^^^^ required by this bound in `take_const_owned`
help: use parentheses to call this type parameter
|
LL | take_const_owned(f());
Copy link
Member Author

@compiler-errors compiler-errors Oct 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is, unfortunately, an erroneous suggestion that is uncovered by the generalizations I made in the last commit in the stack. However, this is not really the fault of the changes I made. For example, a similarly erroneous suggestion already triggers on nightly with this code:

struct Foo;

fn foo() -> Foo {
    todo!()
}

trait Bar {}

impl Bar for Foo {}

fn needs_foo_and_callable<T>(_: impl Bar + Fn() -> T) { }

fn main() {
    needs_foo_and_callable(foo);
}

Results in:

error[E0277]: the trait bound `fn() -> Foo {foo}: Bar` is not satisfied
  --> src/main.rs:14:15
   |
3  | fn foo() -> Foo {
   |    --- consider calling this function
...
14 |     needs_foo_and_callable(foo);
   |     ---------------------- ^^^ the trait `Bar` is not implemented for fn item `fn() -> Foo {foo}`
   |     |
   |     required by a bound introduced by this call
   |
note: required by a bound in `needs_foo_and_callable`
  --> src/main.rs:11:25
   |
11 | fn needs_foo_and_callable<T>(_: impl Bar + Fn() -> T) { }
   |                                      ^^^ required by this bound in `needs_foo`
help: use parentheses to call the function
   |
14 |     needs_foo_and_callable(foo());
   |                               ++

But adding those parentheses causes:

error[E0277]: expected a `Fn<()>` closure, found `Foo`
  --> src/main.rs:14:15
   |
14 |     needs_foo_and_callable(foo());
   |     ---------------------- ^^^^^ expected an `Fn<()>` closure, found `Foo`
   |     |
   |     required by a bound introduced by this call
   |
   = help: the trait `Fn<()>` is not implemented for `Foo`
   = note: wrap the `Foo` in a closure with no arguments: `|| { /* code */ }`
note: required by a bound in `needs_foo_and_callable`
  --> src/main.rs:11:31
   |
11 | fn needs_foo_and_callable<T>(_: impl Bar + Fn() -> T) { }
   |                                            ^^^^^^^^^ required by this bound in `needs_foo`

For more information about this error, try `rustc --explain E0277`.

This is a fundamental limitation to the suggestion being implemented here in trait selection -- I'm happy to brainstorm ways of suppressing this, but I'm happy with keeping it as is and filing a follow-up bug.

Copy link
Member

@nagisa nagisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me, r=me with the wording adjusted.

Somewhat unfortunate that we end up copying a large chunk of code outta typeck but I don’t see a good way to avoid it 🤷

|
LL | fn insert_resource<R: Resource>(resource: R) {}
| ^^^^^^^^ required by this bound in `insert_resource`
help: use parentheses to instantiate this tuple struct
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not entirely sure about using “instantiate” here, due to the parallels with OOP. I think we typically tend towards using “construct”.

@rust-log-analyzer

This comment has been minimized.

@compiler-errors
Copy link
Member Author

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Oct 19, 2022

📌 Commit 35f1570 has been approved by nagisa

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 19, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 19, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#102863 (Standardize "use parentheses to call" suggestions between typeck and trait selection)
 - rust-lang#103034 (Let expressions on RHS shouldn't be terminating scopes)
 - rust-lang#103127 (Make transpose const and inline)
 - rust-lang#103153 (Allow `Vec::leak` when using `no_global_oom_handling`)
 - rust-lang#103182 (Clean up query descriptions)
 - rust-lang#103216 (Consider patterns in fn params in an `Elided(Infer)` lifetime rib.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5c2c476 into rust-lang:master Oct 19, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 19, 2022
@compiler-errors compiler-errors deleted the call-suggestion-on-unimplemented branch November 2, 2022 02:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Poor diagnostic message when passing in type constructor to generic function
6 participants